Replace sprintf with snprintf and strncpy with strlcpy.

There are various cases where no NULL-terminated strings are
guaranteed and eventual possible overflows. This patch fixes them.

From: Christoph Egger <Christoph.Egger@amd.com>
Signed-off-by: Keir Fraser <keir@xensource.com>

diff --git a/xen/arch/x86/cpu/centaur.c b/xen/arch/x86/cpu/centaur.c
index 09e5498c2df7d5833abea5771ed19d114e27bfcc..7744b5b5e5067de63aaba82d9dc7eb09e0d65960 100644 (file)
--- a/xen/arch/x86/cpu/centaur.c
+++ b/xen/arch/x86/cpu/centaur.c
@@ -437,7 +437,8 @@ static void __init init_centaur(struct cpuinfo_x86 *c)
                                /* Add L1 data and code cache sizes. */
                                c->x86_cache_size = (cc>>24)+(dd>>24);
                        }
-                       sprintf( c->x86_model_id, "WinChip %s", name );
+                       snprintf( c->x86_model_id, sizeof(c->x86_model_id),
+                               "WinChip %s", name );
                        break;
 
                case 6:

diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 63237c82f4112ce7b1ac48acf548814fe518a1c6..ae13c7341b357fd1f0e4a6a5125ad5d6f45968cc 100644 (file)
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -386,8 +386,8 @@ void __devinit identify_cpu(struct cpuinfo_x86 *c)
                        strcpy(c->x86_model_id, p);
                else
                        /* Last resort... */
-                       sprintf(c->x86_model_id, "%02x/%02x",
-                               c->x86_vendor, c->x86_model);
+                       snprintf(c->x86_model_id, sizeof(c->x86_model_id),
+                               "%02x/%02x", c->x86_vendor, c->x86_model);
        }
 
        /* Now the feature flags better reflect actual CPU features! */

diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
index 5e505bf4af942990a1b5e6a7009af51320e1b2fd..b32f57861cfd8b98b5b1d2a86f53c40df217ce06 100644 (file)
--- a/xen/arch/x86/domain_build.c
+++ b/xen/arch/x86/domain_build.c
@@ -821,7 +821,7 @@ int construct_dom0(struct domain *d,
     si->pt_base      = vpt_start + 2 * PAGE_SIZE * !!IS_COMPAT(d);
     si->nr_pt_frames = nr_pt_pages;
     si->mfn_list     = vphysmap_start;
-    sprintf(si->magic, "xen-%i.%i-x86_%d%s",
+    snprintf(si->magic, sizeof(si->magic), "xen-%i.%i-x86_%d%s",
             xen_major_version(), xen_minor_version(),
             elf_64bit(&elf) ? 64 : 32,
             parms.pae ? "p" : "");
@@ -871,7 +871,7 @@ int construct_dom0(struct domain *d,
 
     memset(si->cmd_line, 0, sizeof(si->cmd_line));
     if ( cmdline != NULL )
-        strncpy((char *)si->cmd_line, cmdline, sizeof(si->cmd_line)-1);
+        strlcpy((char *)si->cmd_line, cmdline, sizeof(si->cmd_line));
 
     if ( fill_console_start_info((void *)(si + 1)) )
     {

diff --git a/xen/arch/x86/hvm/intercept.c b/xen/arch/x86/hvm/intercept.c
index 9a454c6290a91742bc0779cd0d4fbf33d2a7b743..e5c1728b7dc6954a2ab0322a86f44ff51023bb5b 100644 (file)
--- a/xen/arch/x86/hvm/intercept.c
+++ b/xen/arch/x86/hvm/intercept.c
@@ -173,7 +173,7 @@ int hvm_register_savevm(struct domain *d,
         return -1;
     }
 
-    strncpy(se->idstr, idstr, HVM_SE_IDSTR_LEN);
+    strlcpy(se->idstr, idstr, HVM_SE_IDSTR_LEN);
 
     se->instance_id = instance_id;
     se->version_id = version_id;

diff --git a/xen/arch/x86/oprofile/nmi_int.c b/xen/arch/x86/oprofile/nmi_int.c
index ffefe3bf2de76acdec4884b7a7e6ac6067001dfc..518691337c0dd718abd4e00e9ee867d9089de761 100644 (file)
--- a/xen/arch/x86/oprofile/nmi_int.c
+++ b/xen/arch/x86/oprofile/nmi_int.c
@@ -22,6 +22,7 @@
 #include <asm/regs.h>
 #include <asm/current.h>
 #include <xen/delay.h>
+#include <xen/string.h>
  
 #include "op_counter.h"
 #include "op_x86_model.h"
@@ -39,7 +40,6 @@ extern int is_active(struct domain *d);
 extern int active_id(struct domain *d);
 extern int is_profiled(struct domain *d);
 
-extern size_t strlcpy(char *dest, const char *src, size_t size);
 
 
 static int nmi_callback(struct cpu_user_regs *regs, int cpu)
@@ -276,20 +276,20 @@ static int __init p4_init(char * cpu_type)
        }
 
 #ifndef CONFIG_SMP
-       strncpy (cpu_type, "i386/p4", XENOPROF_CPU_TYPE_SIZE - 1);
+       strlcpy (cpu_type, "i386/p4", XENOPROF_CPU_TYPE_SIZE);
        model = &op_p4_spec;
        return 1;
 #else
        switch (smp_num_siblings) {
                case 1:
-                       strncpy (cpu_type, "i386/p4", 
-                                XENOPROF_CPU_TYPE_SIZE - 1);
+                       strlcpy (cpu_type, "i386/p4", 
+                                XENOPROF_CPU_TYPE_SIZE);
                        model = &op_p4_spec;
                        return 1;
 
                case 2:
-                       strncpy (cpu_type, "i386/p4-ht", 
-                                XENOPROF_CPU_TYPE_SIZE - 1);
+                       strlcpy (cpu_type, "i386/p4-ht", 
+                                XENOPROF_CPU_TYPE_SIZE);
                        model = &op_p4_ht2_spec;
                        return 1;
        }
@@ -311,17 +311,17 @@ static int __init ppro_init(char *cpu_type)
                return 0;
        }
        else if (cpu_model == 15)
-               strncpy (cpu_type, "i386/core_2", XENOPROF_CPU_TYPE_SIZE - 1);
+               strlcpy (cpu_type, "i386/core_2", XENOPROF_CPU_TYPE_SIZE);
        else if (cpu_model == 14)
-               strncpy (cpu_type, "i386/core", XENOPROF_CPU_TYPE_SIZE - 1);
+               strlcpy (cpu_type, "i386/core", XENOPROF_CPU_TYPE_SIZE);
        else if (cpu_model == 9)
-               strncpy (cpu_type, "i386/p6_mobile", XENOPROF_CPU_TYPE_SIZE - 1);
+               strlcpy (cpu_type, "i386/p6_mobile", XENOPROF_CPU_TYPE_SIZE);
        else if (cpu_model > 5)
-               strncpy (cpu_type, "i386/piii", XENOPROF_CPU_TYPE_SIZE - 1);
+               strlcpy (cpu_type, "i386/piii", XENOPROF_CPU_TYPE_SIZE);
        else if (cpu_model > 2)
-               strncpy (cpu_type, "i386/pii", XENOPROF_CPU_TYPE_SIZE - 1);
+               strlcpy (cpu_type, "i386/pii", XENOPROF_CPU_TYPE_SIZE);
        else
-               strncpy (cpu_type, "i386/ppro", XENOPROF_CPU_TYPE_SIZE - 1);
+               strlcpy (cpu_type, "i386/ppro", XENOPROF_CPU_TYPE_SIZE);
 
        model = &op_ppro_spec;
        return 1;
@@ -346,9 +346,6 @@ int nmi_init(int *num_events, int *is_primary, char *cpu_type)
                }
        }
  
-       /* Make sure string is NULL terminated */
-       cpu_type[XENOPROF_CPU_TYPE_SIZE - 1] = 0;
-
        switch (vendor) {
                case X86_VENDOR_AMD:
                        /* Needs to be at least an Athlon (or hammer in 32bit mode) */
@@ -361,15 +358,15 @@ int nmi_init(int *num_events, int *is_primary, char *cpu_type)
                                return -ENODEV;
                        case 6:
                                model = &op_athlon_spec;
-                               strncpy (cpu_type, "i386/athlon", 
-                                        XENOPROF_CPU_TYPE_SIZE - 1);
+                               strlcpy (cpu_type, "i386/athlon", 
+                                        XENOPROF_CPU_TYPE_SIZE);
                                break;
                        case 0xf:
                                model = &op_athlon_spec;
                                /* Actually it could be i386/hammer too, but give
                                   user space an consistent name. */
-                               strncpy (cpu_type, "x86-64/hammer", 
-                                        XENOPROF_CPU_TYPE_SIZE - 1);
+                               strlcpy (cpu_type, "x86-64/hammer", 
+                                        XENOPROF_CPU_TYPE_SIZE);
                                break;
                        }
                        break;

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 96d49203ca7417d6d9440c48e79fea536b9db01f..c8daf1c28e9d4f568f98285a191ae0dd1f34413c 100644 (file)
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -111,8 +111,7 @@ char acpi_param[10] = "";
 static void parse_acpi_param(char *s)
 {
     /* Save the parameter so it can be propagated to domain0. */
-    strncpy(acpi_param, s, sizeof(acpi_param));
-    acpi_param[sizeof(acpi_param)-1] = '\0';
+    strlcpy(acpi_param, s, sizeof(acpi_param));
 
     /* Interpret the parameter for use within Xen. */
     if ( !strcmp(s, "off") )
@@ -804,35 +803,57 @@ void __init __start_xen(multiboot_info_t *mbi)
 void arch_get_xen_caps(xen_capabilities_info_t info)
 {
     char *p = info;
+    int i = 0;
     int major = xen_major_version();
     int minor = xen_minor_version();
 
 #if defined(CONFIG_X86_32) && !defined(CONFIG_X86_PAE)
 
-    p += sprintf(p, "xen-%d.%d-x86_32 ", major, minor);
-    if ( hvm_enabled )
-        p += sprintf(p, "hvm-%d.%d-x86_32 ", major, minor);
+    i = snprintf(p, sizeof(xen_capabilities_info_t),
+                 "xen-%d.%d-x86_32 ", major, minor);
+    p += i;
+    if ( hvm_enabled ) {
+        i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+               "hvm-%d.%d-x86_32 ", major, minor);
+       p += i;
+    }
 
 #elif defined(CONFIG_X86_32) && defined(CONFIG_X86_PAE)
 
-    p += sprintf(p, "xen-%d.%d-x86_32p ", major, minor);
+    i = snprintf(p, sizeof(xen_capabilities_info_t),
+                 "xen-%d.%d-x86_32p ", major, minor);
+    p += i;
     if ( hvm_enabled )
     {
-        p += sprintf(p, "hvm-%d.%d-x86_32 ", major, minor);
-        p += sprintf(p, "hvm-%d.%d-x86_32p ", major, minor);
+        i = snprintf(p, sizeof(xen_capabilities_info_t),
+                     "hvm-%d.%d-x86_32 ", major, minor);
+        p += i;
+        i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+                     "hvm-%d.%d-x86_32p ", major, minor);
+        p += i;
     }
 
 #elif defined(CONFIG_X86_64)
 
-    p += sprintf(p, "xen-%d.%d-x86_64 ", major, minor);
+    i = snprintf(p, sizeof(xen_capabilities_info_t),
+                 "xen-%d.%d-x86_64 ", major, minor);
+    p += i;
 #ifdef CONFIG_COMPAT
-    p += sprintf(p, "xen-%d.%d-x86_32p ", major, minor);
+    i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+                "xen-%d.%d-x86_32p ", major, minor);
+    p += i;
 #endif
     if ( hvm_enabled )
     {
-        p += sprintf(p, "hvm-%d.%d-x86_32 ", major, minor);
-        p += sprintf(p, "hvm-%d.%d-x86_32p ", major, minor);
-        p += sprintf(p, "hvm-%d.%d-x86_64 ", major, minor);
+        i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+                     "hvm-%d.%d-x86_32 ", major, minor);
+        p += i;
+        i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+                     "hvm-%d.%d-x86_32p ", major, minor);
+        p += i;
+        i = snprintf(p, sizeof(xen_capabilities_info_t) - i,
+                     "hvm-%d.%d-x86_64 ", major, minor);
+        p += i;
     }
 
 #else

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 387b18c20a7f152ffc52a3f3bd6aab235e66a046..b57f34f4f02c5ad0098f70ee7d84750cf3d7ac87 100644 (file)
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -274,7 +274,7 @@ static char *freq_string(u64 freq)
     unsigned int x, y;
     y = (unsigned int)do_div(freq, 1000000) / 1000;
     x = (unsigned int)freq;
-    sprintf(s, "%u.%03uMHz", x, y);
+    snprintf(s, sizeof(s), "%u.%03uMHz", x, y);
     return s;
 }
 

diff --git a/xen/common/gdbstub.c b/xen/common/gdbstub.c
index 8c863080e04d7e96f7dbaeab24fbf338245af270..b9dfd08bbc611c05532544db33d7fa68654c0c63 100644 (file)
--- a/xen/common/gdbstub.c
+++ b/xen/common/gdbstub.c
@@ -268,7 +268,7 @@ gdb_send_packet(struct gdb_context *ctx)
     char buf[3];
     int count;
 
-    sprintf(buf, "%.02x\n", ctx->out_csum);
+    snprintf(buf, sizeof(buf), "%.02x\n", ctx->out_csum);
 
     gdb_write_to_packet_char('#', ctx);
     gdb_write_to_packet(buf, 2, ctx);

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index 016270b2c5c38b3a3e225a118ad27561f50989bb..4c22cf8803ef152bbd7b99a75e8fa1a58e1cd4a9 100644 (file)
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -72,8 +72,7 @@ void cmdline_parse(char *cmdline)
             switch ( param->type )
             {
             case OPT_STR:
-                strncpy(param->var, optval, param->len);
-                ((char *)param->var)[param->len-1] = '\0';
+                strlcpy(param->var, optval, param->len);
                 break;
             case OPT_UINT:
                 *(unsigned int *)param->var =

diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 244273addfbbb6ae2935d0763cdceafb67b9a3e9..5ff45d0d54982a8f9085b552d2d1b26a439179f9 100644 (file)
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -67,7 +67,7 @@ void register_keyhandler(
     ASSERT(key_table[key].u.handler == NULL);
     key_table[key].u.handler = handler;
     key_table[key].flags     = 0;
-    strncpy(key_table[key].desc, desc, STR_MAX);
+    strlcpy(key_table[key].desc, desc, STR_MAX);
     key_table[key].desc[STR_MAX-1] = '\0';
 }
 
@@ -77,8 +77,7 @@ void register_irq_keyhandler(
     ASSERT(key_table[key].u.irq_handler == NULL);
     key_table[key].u.irq_handler = handler;
     key_table[key].flags         = KEYHANDLER_IRQ_CALLBACK;
-    strncpy(key_table[key].desc, desc, STR_MAX);
-    key_table[key].desc[STR_MAX-1] = '\0';
+    strlcpy(key_table[key].desc, desc, STR_MAX);
 }
 
 static void show_handlers(unsigned char key)

diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c
index c19339ab0139a2c9a29fb84a88464ac9cf675e23..d3ddcdb6fe678d7dde3111ba689289e06ef7779b 100644 (file)
--- a/xen/common/libelf/libelf-dominfo.c
+++ b/xen/common/libelf/libelf-dominfo.c
@@ -128,16 +128,16 @@ int elf_xen_parse_note(struct elf_binary *elf,
     switch (type)
     {
     case XEN_ELFNOTE_LOADER:
-       strncpy(parms->loader, str, sizeof(parms->loader));
+       strlcpy(parms->loader, str, sizeof(parms->loader));
        break;
     case XEN_ELFNOTE_GUEST_OS:
-       strncpy(parms->guest_os, str, sizeof(parms->guest_os));
+       strlcpy(parms->guest_os, str, sizeof(parms->guest_os));
        break;
     case XEN_ELFNOTE_GUEST_VERSION:
-       strncpy(parms->guest_ver, str, sizeof(parms->guest_ver));
+       strlcpy(parms->guest_ver, str, sizeof(parms->guest_ver));
        break;
     case XEN_ELFNOTE_XEN_VERSION:
-       strncpy(parms->xen_ver, str, sizeof(parms->xen_ver));
+       strlcpy(parms->xen_ver, str, sizeof(parms->xen_ver));
        break;
     case XEN_ELFNOTE_PAE_MODE:
        if (0 == strcmp(str, "yes"))
@@ -224,13 +224,13 @@ int elf_xen_parse_guest_info(struct elf_binary *elf,
 
        /* strings */
        if (0 == strcmp(name, "LOADER"))
-           strncpy(parms->loader, value, sizeof(parms->loader));
+           strlcpy(parms->loader, value, sizeof(parms->loader));
        if (0 == strcmp(name, "GUEST_OS"))
-           strncpy(parms->guest_os, value, sizeof(parms->guest_os));
+           strlcpy(parms->guest_os, value, sizeof(parms->guest_os));
        if (0 == strcmp(name, "GUEST_VER"))
-           strncpy(parms->guest_ver, value, sizeof(parms->guest_ver));
+           strlcpy(parms->guest_ver, value, sizeof(parms->guest_ver));
        if (0 == strcmp(name, "XEN_VER"))
-           strncpy(parms->xen_ver, value, sizeof(parms->xen_ver));
+           strlcpy(parms->xen_ver, value, sizeof(parms->xen_ver));
        if (0 == strcmp(name, "PAE"))
        {
            if (0 == strcmp(value, "yes[extended-cr3]"))

diff --git a/xen/common/perfc.c b/xen/common/perfc.c
index 5ab8c6e9f3344d108b1359ee2f7f729d29f1c93f..86fda48e07185d721ffec03f05caecad37bc1e83 100644 (file)
--- a/xen/common/perfc.c
+++ b/xen/common/perfc.c
@@ -148,9 +148,8 @@ static int perfc_copy_info(XEN_GUEST_HANDLE_64(xen_sysctl_perfc_desc_t) desc,
     {
         for ( i = 0; i < NR_PERFCTRS; i++ )
         {
-            strncpy(perfc_d[i].name, perfc_info[i].name,
+            strlcpy(perfc_d[i].name, perfc_info[i].name,
                     sizeof(perfc_d[i].name));
-            perfc_d[i].name[sizeof(perfc_d[i].name)-1] = '\0';
 
             switch ( perfc_info[i].type )
             {

diff --git a/xen/common/rangeset.c b/xen/common/rangeset.c
index d90a39b67b59daa92fcbe4b203e2fc2916a5a56a..3594db41e99fcb8a44939fba87aee991e200eeee 100644 (file)
--- a/xen/common/rangeset.c
+++ b/xen/common/rangeset.c
@@ -283,12 +283,11 @@ struct rangeset *rangeset_new(
 
     if ( name != NULL )
     {
-        strncpy(r->name, name, sizeof(r->name));
-        r->name[sizeof(r->name)-1] = '\0';
+        strlcpy(r->name, name, sizeof(r->name));
     }
     else
     {
-        sprintf(r->name, "(no name)");
+        snprintf(r->name, sizeof(r->name), "(no name)");
     }
 
     if ( (r->domain = d) != NULL )

diff --git a/xen/common/symbols.c b/xen/common/symbols.c
index fba6cf0867dac6bc667cf3858ffbca29e3fc5f96..f4134b7ed5e525e3bddc1ffd61e54c951597927a 100644 (file)
--- a/xen/common/symbols.c
+++ b/xen/common/symbols.c
@@ -142,15 +142,17 @@ void __print_symbol(const char *fmt, unsigned long address)
     const char *name;
     unsigned long offset, size;
     char namebuf[KSYM_NAME_LEN+1];
-    char buffer[sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN +
-               2*(BITS_PER_LONG*3/10) + 1];
+
+#define BUFFER_SIZE sizeof("%s+%#lx/%#lx [%s]") + KSYM_NAME_LEN + \
+                       2*(BITS_PER_LONG*3/10) + 1
+    char buffer[BUFFER_SIZE];
 
     name = symbols_lookup(address, &size, &offset, namebuf);
 
     if (!name)
-        sprintf(buffer, "???");
+        snprintf(buffer, BUFFER_SIZE, "???");
     else
-        sprintf(buffer, "%s+%#lx/%#lx", name, offset, size);
+        snprintf(buffer, BUFFER_SIZE, "%s+%#lx/%#lx", name, offset, size);
 
     printk(fmt, buffer);
 }